Skip to content

Add Electron desktop app for Windows - #112

Open
starknetdev wants to merge 8 commits into
mainfrom
feature/electron-desktop
Open

Add Electron desktop app for Windows#112
starknetdev wants to merge 8 commits into
mainfrom
feature/electron-desktop

Conversation

@starknetdev

@starknetdev starknetdev commented Mar 18, 2026

Copy link
Copy Markdown
Member

Summary

  • Adds Electron wrapper for native Windows desktop builds (targeting Epic Games Store)
  • App loads directly to /trials route and blocks navigation to other routes
  • Includes GitHub Actions workflow to build Windows .exe on pushes to main
  • Fixes desktop Exit Game button navigating to / instead of current dungeon

What's changed

  • client/electron/ — Main process, preload script, and TS config
  • client/package.json — Electron devDependencies, scripts, and electron-builder config
  • client/src/utils/utils.ts — Added isElectron() utility (unused, available for future use)
  • client/src/desktop/overlays/Settings.tsx — Exit Game navigates to dungeon landing instead of /
  • .github/workflows/electron-build.yml — Windows build CI

What's NOT changed

  • vite.config.ts — No changes from main (relative base was added then removed)
  • Wallet/connector code — Cartridge Controller works as-is in Electron
  • CSS/layout — Desktop layout already works
  • Web app behavior — No regression, all changes are additive or Electron-only

Test plan

  • pnpm dev still works normally (no web regression)
  • Download Windows artifact from CI, extract, run Loot Survivor 2.exe
  • App opens to /trials route
  • Exit Game button returns to /trials landing page
  • Wallet connection works in Electron window

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Desktop Windows packaging and distribution support (Electron) with a packaged local server for production.
    • Renderer can detect running in the desktop app.
  • Build

    • Automated Windows build workflow added.
    • New development and packaging scripts for the desktop app.
  • Analytics

    • Analytics now include platform and app-host details.
  • Bug Fixes

    • Exiting Settings now returns to the current dungeon view.

starknetdev and others added 7 commits March 17, 2026 14:51
Adds a thin Electron shell for native macOS/Windows/Linux desktop builds.
Cartridge Controller works as-is in Electron's Chromium environment.

- electron/main.ts: BrowserWindow with dev/prod loading
- electron/preload.ts: exposes isElectron flag via contextBridge
- electron/tsconfig.json: Node.js-targeted TS config
- New scripts: electron:dev, electron:build, electron:preview
- vite base set to './' for file:// protocol compatibility
- isElectron() utility added to utils.ts

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- electron/main.ts: loads /trials directly, blocks navigation to other
  routes, uses local HTTP server in prod for BrowserRouter compatibility
- package.json: Windows-only dir target for Epic Games Store, added
  @types/node
- Added GitHub Actions workflow to build Windows .exe on push to branch
  and upload as artifact

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Use process.resourcesPath for correct dist path in packaged builds
- Use app.isPackaged instead of NODE_ENV for dev/prod detection
- Add error handling on app startup
- Add typeRoots to electron tsconfig for @types/node resolution

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
electron-builder extraMetadata overrides "type" to "commonjs" in the
packaged package.json, preventing the "exports is not defined" error
caused by Node treating CommonJS output as ESM.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Use app.getAppPath() instead of process.resourcesPath - works correctly
whether the app is packed with asar or unpacked.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Desktop Settings overlay was navigating to '/' instead of the current
dungeon route (e.g. /trials). Now matches mobile behavior.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Only rebuilds when client/ files change.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@vercel

vercel Bot commented Mar 18, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
loot-survivor-2 Ready Ready Preview, Comment Mar 25, 2026 10:05am

Request Review

@coderabbitai

coderabbitai Bot commented Mar 18, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

Adds Electron desktop support: an Electron main process and preload script, a local static server for packaged assets, build/package configuration and GitHub Actions workflow for Windows builds, Vite base path change, renderer Electron-detection utility, and analytics/platform query handling.

Changes

Cohort / File(s) Summary
CI / Packaging Workflow
.github/workflows/electron-build.yml
New GitHub Actions workflow to build Windows Electron artifacts with a matrix over app_host (epic, steam) and upload per-variant artifacts.
Electron TypeScript & Build
client/electron/tsconfig.json, client/package.json, client/.gitignore
Added tsconfig for electron files, electron-related npm scripts, build block (appId, productName, Windows target), devDependencies (electron, electron-builder, etc.), pnpm setting, and ignored electron-dist/release.
Electron Runtime
client/electron/main.ts, client/electron/preload.ts
New main process creating BrowserWindow with contextIsolation and preload, SPA navigation guard limiting routes to /trials, dev vs production asset loading (localhost or local HTTP server), and preload exposing electronAPI.isElectron.
Frontend config
client/vite.config.ts
Set Vite base to "./" (relative base path) to support packaged/relative asset resolution.
Runtime utilities & integration
client/src/utils/utils.ts, client/src/utils/platform.ts, client/src/Main.tsx, client/src/utils/analytics.ts, client/src/desktop/overlays/Settings.tsx
Added isElectron() detection helper and getPlatform()/getAppHost() functions (query param and env fallbacks); PostHog init extended with persistence, bootstrap flags and loaded callback; analytics identify enriched with platform and app_host; Settings navigation now routes to current dungeon path.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Electron as Electron<br/>(Main)
    participant LocalServer as Local HTTP<br/>Server
    participant Renderer as Renderer<br/>Process
    participant Dist as Dist Assets

    User->>Electron: Start app
    Electron->>Electron: Determine dev vs prod
    alt Development
        Electron->>Renderer: Load http://localhost:5173/trials
    else Production
        Electron->>LocalServer: Start serving `dist` on dynamic port
        LocalServer->>Dist: Read files
        Electron->>Renderer: Load http://127.0.0.1:port/trials
    end

    Renderer->>Renderer: Check `window.electronAPI.isElectron`
    Renderer->>Electron: will-navigate events (on navigation)
    Electron->>Electron: Validate path within `/trials` and allow/block
    Renderer->>LocalServer: GET /asset (prod)
    LocalServer->>Dist: Serve file with MIME
    LocalServer->>Renderer: Respond with asset
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I tunneled code from web to app,
A preload flag and local map,
Guarded paths and builds that hum,
Windows artifacts — here they come!
Hop, compile, and package — yum! 🥕✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly matches the PR's primary objective: adding Electron support for building a native Windows desktop application.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/electron-desktop

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@claude

claude Bot commented Mar 18, 2026

Copy link
Copy Markdown

Claude finished @starknetdev's task —— View job


Code Review for Electron Desktop App

Starting comprehensive review of the Electron desktop implementation...

Review Checklist

  • Analyze Electron main process implementation
  • Review preload script security
  • Evaluate CI/CD workflow configuration
  • Check package.json dependencies and scripts
  • Review desktop UI modifications
  • Assess security considerations
  • Check for potential bugs and performance issues

@gemini-code-assist

Copy link
Copy Markdown

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request integrates an Electron framework to create a native Windows desktop application for "Loot Survivor 2". The primary goal is to provide a dedicated desktop experience, starting the application directly at the '/trials' route and restricting navigation to maintain a controlled environment. This also includes necessary build configurations and a fix for in-app navigation within the desktop settings.

Highlights

  • Electron Desktop App: Introduced an Electron wrapper for native Windows desktop builds, specifically targeting the Epic Games Store.
  • Route Management: Configured the application to load directly to the '/trials' route and block navigation to other routes, ensuring a focused experience.
  • Build Configuration: Added Electron development scripts, dependencies, and 'electron-builder' configuration for packaging the desktop application.
  • Navigation Fix: Corrected the 'Exit Game' button in desktop settings to navigate to the current dungeon's landing page instead of the root.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Ignored Files
  • Ignored by pattern: .github/workflows/** (1)
    • .github/workflows/electron-build.yml
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds an Electron wrapper for the application, which is a significant feature addition. The implementation is generally solid, but I've identified a few areas for improvement. My main concerns are a broken dependency version in package.json which will prevent installation, and the use of synchronous file I/O in the Electron main process which can affect performance and responsiveness. I've also included suggestions to improve error handling and type safety for better maintainability.

Comment thread client/package.json
"vite-plugin-wasm": "^3.4.1"
},
"devDependencies": {
"@types/node": "^25.5.0",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The specified version ^25.5.0 for @types/node does not exist. The latest major version is 20. This will likely cause dependency installation to fail. Please use a valid and existing version.

Suggested change
"@types/node": "^25.5.0",
"@types/node": "^20.14.0",

Comment thread client/electron/main.ts
Comment on lines +37 to +54
const server = http.createServer((req, res) => {
const url = new URL(req.url || "/", "http://localhost");
let filePath = path.join(distPath, url.pathname);

// Serve index.html for SPA routes
if (!fs.existsSync(filePath) || fs.statSync(filePath).isDirectory()) {
filePath = path.join(distPath, "index.html");
}

try {
const data = fs.readFileSync(filePath);
res.writeHead(200, { "Content-Type": getMimeType(filePath) });
res.end(data);
} catch {
res.writeHead(404);
res.end("Not found");
}
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The local server is using synchronous file system calls (existsSync, statSync, readFileSync) within the request handler. This will block the Electron main process for every file request, which can lead to unresponsiveness of the application, especially when loading many assets. It's better to use the asynchronous versions of these methods from fs.promises and make the request handler async.

    const server = http.createServer(async (req, res) => {
      const url = new URL(req.url || "/", "http://localhost");
      let filePath = path.join(distPath, url.pathname);

      // Serve index.html for SPA routes
      try {
        const stats = await fs.promises.stat(filePath);
        if (stats.isDirectory()) {
          filePath = path.join(distPath, "index.html");
        }
      } catch {
        // If stat fails, file likely doesn't exist, so serve index.html for SPA.
        filePath = path.join(distPath, "index.html");
      }

      try {
        const data = await fs.promises.readFile(filePath);
        res.writeHead(200, { "Content-Type": getMimeType(filePath) });
        res.end(data);
      } catch (err) {
        console.error(`Failed to serve file ${filePath}:`, err);
        res.writeHead(404);
        res.end("Not found");
      }
    });

Comment thread client/electron/main.ts
Comment on lines +86 to +88
} catch {
event.preventDefault();
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This catch block silently prevents navigation if parsing the URL fails. For easier debugging, it's a good practice to log the error that was caught.

    } catch (error) {
      console.error(`Failed to parse navigation URL: ${url}`, error);
      event.preventDefault();
    }

Comment thread client/src/utils/utils.ts
Comment on lines +4 to +6
export const isElectron = (): boolean =>
typeof window !== "undefined" &&
!!(window as any).electronAPI?.isElectron;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using (window as any) bypasses TypeScript's type safety. A better approach is to augment the global Window interface to include the electronAPI. This provides type safety and autocompletion in your IDE.

You can do this by creating a declaration file (e.g., src/electron.d.ts) with the following content:

declare global {
  interface Window {
    electronAPI?: {
      isElectron: boolean;
    };
  }
}
// This empty export is needed to make the file a module.
export {};

Make sure this file is included in your tsconfig.json. Then you can update the isElectron function to be type-safe without using any.

Suggested change
export const isElectron = (): boolean =>
typeof window !== "undefined" &&
!!(window as any).electronAPI?.isElectron;
export const isElectron = (): boolean =>
typeof window !== "undefined" &&
!!window.electronAPI?.isElectron;

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (3)
client/electron/main.ts (1)

35-62: Consider cleaning up the HTTP server on app quit.

The local HTTP server is started but never explicitly closed when the app quits. While this is minor since the process terminates anyway, explicitly closing the server is a best practice for graceful shutdown.

Optional: Add server cleanup
+let localServer: http.Server | null = null;
+
 function startLocalServer(distPath: string): Promise<number> {
   return new Promise((resolve) => {
-    const server = http.createServer((req, res) => {
+    localServer = http.createServer((req, res) => {
       // ... existing code
     });
 
-    server.listen(0, "127.0.0.1", () => {
-      const addr = server.address();
+    localServer.listen(0, "127.0.0.1", () => {
+      const addr = localServer!.address();
       const port = typeof addr === "object" && addr ? addr.port : 0;
       resolve(port);
     });
   });
 }

 app.on("window-all-closed", () => {
+  if (localServer) {
+    localServer.close();
+  }
   if (process.platform !== "darwin") {
     app.quit();
   }
 });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@client/electron/main.ts` around lines 35 - 62, startLocalServer currently
creates an HTTP server but never closes it; modify startLocalServer to expose
the server instance (e.g., return both the port and the server or return the
server and resolve the port via server.address()) so the caller can call
server.close(), then register a quit handler in the Electron lifecycle (e.g.,
app.on('before-quit'/'quit')) to call server.close() and handle any
callback/error from server.close(); update references where startLocalServer is
called to close the returned server on app shutdown.
client/src/utils/utils.ts (1)

4-6: Consider adding a type declaration for electronAPI.

The (window as any) cast works but loses type safety. Adding a type declaration would improve developer experience and catch typos.

Optional: Add type declaration

Create a type declaration file or add to an existing one:

// client/src/types/electron.d.ts
interface ElectronAPI {
  isElectron: boolean;
}

declare global {
  interface Window {
    electronAPI?: ElectronAPI;
  }
}

export {};

Then simplify the utility:

-export const isElectron = (): boolean =>
-  typeof window !== "undefined" &&
-  !!(window as any).electronAPI?.isElectron;
+export const isElectron = (): boolean =>
+  typeof window !== "undefined" &&
+  !!window.electronAPI?.isElectron;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@client/src/utils/utils.ts` around lines 4 - 6, The isElectron utility
currently uses (window as any).electronAPI which sacrifices type safety; add a
type declaration for ElectronAPI and extend the global Window interface (e.g.,
interface ElectronAPI { isElectron: boolean } and declare global { interface
Window { electronAPI?: ElectronAPI } }) so you can replace the cast and
reference window.electronAPI?.isElectron directly in the isElectron function;
update or add a .d.ts file (client/src/types/electron.d.ts) and then simplify
the isElectron export to use the typed window.electronAPI.
.github/workflows/electron-build.yml (1)

35-37: Consider explicitly setting NODE_ENV for the Vite build.

While vite build defaults to production mode, explicitly setting NODE_ENV=production makes the intent clear and guards against any custom config that might check this variable.

♻️ Suggested improvement
      - name: Build Vite
        working-directory: client
+       env:
+         NODE_ENV: production
        run: npx vite build
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/electron-build.yml around lines 35 - 37, The GitHub
Actions step named "Build Vite" currently runs "npx vite build" without an
explicit NODE_ENV; update that step to set NODE_ENV=production for the build
(e.g., via environment key or by prefixing the run command) so the "Build Vite"
step (working-directory: client, run: npx vite build) always runs with
NODE_ENV=production and prevents custom tooling from misdetecting the
environment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@client/package.json`:
- Line 13: The "electron:dev" npm script uses POSIX env syntax
("NODE_ENV=development electron .") which breaks on Windows; add cross-env as a
devDependency and update the "electron:dev" script to prefix the command with
cross-env so the environment variable is set cross-platform (install cross-env
in devDependencies and change the script value referenced as "electron:dev" to
use cross-env NODE_ENV=development electron .).

In `@client/src/desktop/overlays/Settings.tsx`:
- Around line 23-25: The Exit Game path `/${dungeon?.id}` will be blocked by the
Electron navigation guard; modify handleExitGame so that when running in
Electron it navigates to `/trials` instead of `/${dungeon?.id ?? ''}`. Update
the handleExitGame function to detect Electron (e.g., check
navigator.userAgent.includes('Electron') or a platform flag exposed on window)
and call navigate('/trials') in that case, otherwise keep the existing
navigate(`/${dungeon?.id ?? ''}`) behavior.

---

Nitpick comments:
In @.github/workflows/electron-build.yml:
- Around line 35-37: The GitHub Actions step named "Build Vite" currently runs
"npx vite build" without an explicit NODE_ENV; update that step to set
NODE_ENV=production for the build (e.g., via environment key or by prefixing the
run command) so the "Build Vite" step (working-directory: client, run: npx vite
build) always runs with NODE_ENV=production and prevents custom tooling from
misdetecting the environment.

In `@client/electron/main.ts`:
- Around line 35-62: startLocalServer currently creates an HTTP server but never
closes it; modify startLocalServer to expose the server instance (e.g., return
both the port and the server or return the server and resolve the port via
server.address()) so the caller can call server.close(), then register a quit
handler in the Electron lifecycle (e.g., app.on('before-quit'/'quit')) to call
server.close() and handle any callback/error from server.close(); update
references where startLocalServer is called to close the returned server on app
shutdown.

In `@client/src/utils/utils.ts`:
- Around line 4-6: The isElectron utility currently uses (window as
any).electronAPI which sacrifices type safety; add a type declaration for
ElectronAPI and extend the global Window interface (e.g., interface ElectronAPI
{ isElectron: boolean } and declare global { interface Window { electronAPI?:
ElectronAPI } }) so you can replace the cast and reference
window.electronAPI?.isElectron directly in the isElectron function; update or
add a .d.ts file (client/src/types/electron.d.ts) and then simplify the
isElectron export to use the typed window.electronAPI.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d64c4b22-ed93-48c5-b270-0ffac3d779a9

📥 Commits

Reviewing files that changed from the base of the PR and between 0523f31 and 62243e1.

⛔ Files ignored due to path filters (1)
  • client/pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (9)
  • .github/workflows/electron-build.yml
  • client/.gitignore
  • client/electron/main.ts
  • client/electron/preload.ts
  • client/electron/tsconfig.json
  • client/package.json
  • client/src/desktop/overlays/Settings.tsx
  • client/src/utils/utils.ts
  • client/vite.config.ts

Comment thread client/package.json
"preview": "vite preview",
"serve": "vite preview"
"serve": "vite preview",
"electron:dev": "concurrently \"vite\" \"wait-on http://localhost:5173 && NODE_ENV=development electron .\"",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Cross-platform compatibility issue with NODE_ENV=development.

The inline environment variable syntax NODE_ENV=development electron . works on Unix-like systems but not on Windows CMD/PowerShell natively. Since this PR targets Windows builds, consider using cross-env for cross-platform compatibility.

Suggested fix using cross-env

Add cross-env to devDependencies and update the script:

-"electron:dev": "concurrently \"vite\" \"wait-on http://localhost:5173 && NODE_ENV=development electron .\""
+"electron:dev": "concurrently \"vite\" \"wait-on http://localhost:5173 && cross-env NODE_ENV=development electron .\""
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@client/package.json` at line 13, The "electron:dev" npm script uses POSIX env
syntax ("NODE_ENV=development electron .") which breaks on Windows; add
cross-env as a devDependency and update the "electron:dev" script to prefix the
command with cross-env so the environment variable is set cross-platform
(install cross-env in devDependencies and change the script value referenced as
"electron:dev" to use cross-env NODE_ENV=development electron .).

Comment on lines 23 to 25
const handleExitGame = () => {
navigate('/');
navigate(`/${dungeon?.id ?? ''}`);
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Exit Game navigation will be silently blocked in Electron.

The navigation guard in client/electron/main.ts (lines 76-89) only allows paths starting with /trials or exactly /. When dungeon?.id has a value (e.g., "survivor", "1"), the resulting path /${dungeon.id} will be blocked by event.preventDefault(), causing the Exit Game button to silently fail.

Either:

  1. Update the navigation guard in main.ts to allow the dungeon landing paths, or
  2. Navigate to /trials in the Electron context
Option 1: Update the navigation to use /trials in Electron
+import { isElectron } from '@/utils/utils';
+
 const handleExitGame = () => {
-  navigate(`/${dungeon?.id ?? ''}`);
+  navigate(isElectron() ? '/trials' : `/${dungeon?.id ?? ''}`);
 };
Option 2: Update the Electron guard to allow dungeon paths

In client/electron/main.ts, expand the allowed paths:

+const ALLOWED_DUNGEON_IDS = ["survivor", "trials"]; // Add other valid dungeon IDs
+
 win.webContents.on("will-navigate", (event, url) => {
   try {
     const parsed = new URL(url);
     if (
       !parsed.pathname.startsWith(ALLOWED_PATH_PREFIX) &&
-      parsed.pathname !== "/"
+      parsed.pathname !== "/" &&
+      !ALLOWED_DUNGEON_IDS.some(id => parsed.pathname === `/${id}`)
     ) {
       event.preventDefault();
     }
   } catch {
     event.preventDefault();
   }
 });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@client/src/desktop/overlays/Settings.tsx` around lines 23 - 25, The Exit Game
path `/${dungeon?.id}` will be blocked by the Electron navigation guard; modify
handleExitGame so that when running in Electron it navigates to `/trials`
instead of `/${dungeon?.id ?? ''}`. Update the handleExitGame function to detect
Electron (e.g., check navigator.userAgent.includes('Electron') or a platform
flag exposed on window) and call navigate('/trials') in that case, otherwise
keep the existing navigate(`/${dungeon?.id ?? ''}`) behavior.

Register platform and app_host as super properties on PostHog init so
every event is automatically tagged with its distribution channel.
Iframe hosts pass app_host via URL param; desktop/native builds set it
at build time. Electron CI now uses a matrix to produce separate Epic
and Steam artifacts with the correct app_host baked in.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Mar 25, 2026

Copy link
Copy Markdown

Claude finished @starknetdev's task —— View job


Code Review for Electron Desktop App

Comprehensive review of the Electron desktop implementation completed.

Review Summary ✅

Overall Assessment: The Electron implementation is well-structured and follows security best practices. The code quality is good with proper context isolation and navigation controls. However, there are several issues that should be addressed before merging.


🔴 Critical Issues

1. Navigation Bug in Settings (client/src/desktop/overlays/Settings.tsx:24)

The Exit Game button will be silently blocked by the Electron navigation guard.

Problem: The current navigation guard in main.ts only allows paths starting with /trials or exactly /. When dungeon?.id has a value, the path /${dungeon.id} will be blocked by event.preventDefault().

Solution: Update the handleExitGame function to navigate to /trials when running in Electron:

import { isElectron } from '@/utils/utils';

const handleExitGame = () => {
  navigate(isElectron() ? '/trials' : `/${dungeon?.id ?? ''}`);
};

2. Invalid Dependency Version (client/package.json:84)

The @types/node version ^25.5.0 does not exist and will cause installation failures.

Solution: Update to a valid version:

"@types/node": "^20.14.0"

🟡 Performance & Quality Issues

3. Blocking File Operations (client/electron/main.ts:42-53)

The HTTP server uses synchronous file system operations (existsSync, statSync, readFileSync) which can block the main process and cause UI freezes.

Recommendation: Replace with async operations:

const server = http.createServer(async (req, res) => {
  // Use fs.promises.stat, fs.promises.readFile instead
});

4. Cross-Platform Script Issue (client/package.json:13)

The NODE_ENV=development syntax in the electron:dev script works on Unix but fails on Windows.

Solution: Add cross-env dependency and update the script:

"electron:dev": "concurrently \"vite\" \"wait-on http://localhost:5173 && cross-env NODE_ENV=development electron .\""

5. Type Safety Issue (client/src/utils/utils.ts:6)

Using (window as any) bypasses TypeScript's type safety.

Recommendation: Create a proper type declaration file for better type safety and IDE support.


✅ Security Assessment

Good practices observed:

  • ✅ Context isolation enabled (contextIsolation: true)
  • ✅ Node integration disabled (nodeIntegration: false)
  • ✅ Proper preload script with minimal API exposure
  • ✅ Navigation guard restricting routes to /trials/* and /
  • ✅ Local HTTP server bound to loopback interface only

Security considerations:

  • ✅ No unnecessary APIs exposed to renderer process
  • ✅ Proper file serving with MIME type detection
  • ✅ Route restrictions prevent navigation to unwanted pages

📋 Other Observations

Code Quality

  • Architecture: Well-structured separation between main and renderer processes
  • Error Handling: Basic error handling present but could be enhanced with logging
  • Configuration: Proper Electron builder configuration for Windows deployment

CI/CD Workflow

  • Build Process: Comprehensive workflow covering TypeScript compilation, Vite build, and Electron packaging
  • Artifacts: Proper artifact uploading with retention policy
  • Triggers: Appropriate triggers for main branch pushes

Test Coverage

  • Missing: No tests found for Electron-specific functionality
  • Recommendation: Consider adding basic integration tests for the Electron wrapper

🎯 Recommended Actions

  1. Fix Critical Issues: Address the navigation bug and dependency version before merging
  2. Performance: Consider async file operations for better responsiveness
  3. Cross-Platform: Add cross-env for Windows compatibility
  4. Type Safety: Add proper TypeScript declarations
  5. Testing: Consider adding Electron-specific tests
  6. Documentation: Update README with Electron development instructions

The core implementation is solid and follows Electron best practices. Once the critical issues are resolved, this will be a robust desktop application.


@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@client/src/utils/platform.ts`:
- Around line 4-11: The two helpers are inconsistent: one treats an empty
app_host as absent (using params.get and checking truthiness) while getPlatform
uses params.has("app_host") so ?app_host= yields platform="iframe" but no host.
Update getPlatform to check the actual value (use params.get("app_host") and
test for a non-empty string) so both helpers consistently treat empty app_host
as absent; reference the existing params.get("app_host") usage and the
getPlatform function when applying the fix.
- Line 1: The module currently reads window.location at import time via the
params constant which can fail in non-browser runtimes; change to compute
URLSearchParams lazily inside a function (e.g., export a getParams or
getSearchParams helper) and guard with typeof window !== "undefined" before
accessing window.location, returning an empty URLSearchParams or null when
window is unavailable; replace all direct uses of the top-level params variable
with calls to this new helper.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 46b91458-87d9-4778-9fb1-4dec23680b28

📥 Commits

Reviewing files that changed from the base of the PR and between 62243e1 and 66d2fdf.

📒 Files selected for processing (4)
  • .github/workflows/electron-build.yml
  • client/src/Main.tsx
  • client/src/utils/analytics.ts
  • client/src/utils/platform.ts
✅ Files skipped from review due to trivial changes (1)
  • .github/workflows/electron-build.yml

@@ -0,0 +1,12 @@
const params = new URLSearchParams(window.location.search);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Avoid window access at import time.

Line 1 reads window.location during module evaluation; this can crash in non-browser runtimes before initialization. Compute params inside functions with a typeof window !== "undefined" guard.

Suggested fix
-const params = new URLSearchParams(window.location.search);
+function getSearchParams(): URLSearchParams {
+  if (typeof window === "undefined") return new URLSearchParams("");
+  return new URLSearchParams(window.location.search);
+}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@client/src/utils/platform.ts` at line 1, The module currently reads
window.location at import time via the params constant which can fail in
non-browser runtimes; change to compute URLSearchParams lazily inside a function
(e.g., export a getParams or getSearchParams helper) and guard with typeof
window !== "undefined" before accessing window.location, returning an empty
URLSearchParams or null when window is unavailable; replace all direct uses of
the top-level params variable with calls to this new helper.

Comment on lines +4 to +11
const fromUrl = params.get("app_host");
if (fromUrl) return fromUrl;
return import.meta.env.VITE_PUBLIC_APP_HOST || "direct";
}

export function getPlatform(): string {
if (params.has("app_host")) return "iframe";
return import.meta.env.VITE_PUBLIC_PLATFORM || "web";

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Handle empty app_host consistently across both helpers.

Line 10 uses has("app_host"), while Line 4/5 treats empty values as absent. ?app_host= currently yields inconsistent analytics tags (platform="iframe" with fallback app_host).

Suggested fix
 export function getAppHost(): string {
-  const fromUrl = params.get("app_host");
-  if (fromUrl) return fromUrl;
+  const fromUrl = getSearchParams().get("app_host")?.trim();
+  if (fromUrl) return fromUrl;
   return import.meta.env.VITE_PUBLIC_APP_HOST || "direct";
 }
 
 export function getPlatform(): string {
-  if (params.has("app_host")) return "iframe";
+  const fromUrl = getSearchParams().get("app_host")?.trim();
+  if (fromUrl) return "iframe";
   return import.meta.env.VITE_PUBLIC_PLATFORM || "web";
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@client/src/utils/platform.ts` around lines 4 - 11, The two helpers are
inconsistent: one treats an empty app_host as absent (using params.get and
checking truthiness) while getPlatform uses params.has("app_host") so ?app_host=
yields platform="iframe" but no host. Update getPlatform to check the actual
value (use params.get("app_host") and test for a non-empty string) so both
helpers consistently treat empty app_host as absent; reference the existing
params.get("app_host") usage and the getPlatform function when applying the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant